Skip to content

Fix what only worked on Linux, and a CRLF level file bug - #21

Open
Upabjojr wants to merge 5 commits into
tomluchowski:shaders-improvementfrom
Upabjojr:split/cross-platform-fixes
Open

Fix what only worked on Linux, and a CRLF level file bug#21
Upabjojr wants to merge 5 commits into
tomluchowski:shaders-improvementfrom
Upabjojr:split/cross-platform-fixes

Conversation

@Upabjojr

@Upabjojr Upabjojr commented Aug 1, 2026

Copy link
Copy Markdown

Fixes for everything that only ever worked on Linux, plus one level-file bug it hid:

  • CRLF level files (the one fix verified from Linux): a level saved on Windows failed to load anywhere else because only Windows strips the carriage return. Helper::readFile now strips it, covering every level and config file.
  • The macOS branch of ResourceManager::setupDataPath() did not compile (char-array + char-array pointer arithmetic).
  • The editor's load/save dialogs started in $HOME, unset on Windows; they now start in the per-platform user levels folder.
  • The window icon used SetClassLong with a handle cast to LONG, which does not compile in a 64-bit Windows build; now SetClassLongPtr.
  • The Discord link ran xdg-open unconditionally; now ShellExecute on Windows and open on macOS.
  • The editor's "show hidden files" checkbox did nothing on Windows (dot-name check); it now reads the file attribute.
  • setupOgreResources treated any path not starting with / as relative, and folder-separator checks only accepted /.

One hunk of the original second commit is not here: the separator fix inside setupDefaultDataPath(), a function this branch does not have — it is carried by the user-data-folder PR instead.

Note for merge order: trivial adjacent-lines conflicts with the user-data-folder PR (ResourceManager.cpp) and the editor-improvements PR (EditorMode.cpp, the removed getEnv() region).


Split out of #16 so each topic can be reviewed on its own. Merging all of the split PRs reproduces the tree of #16 exactly.

🤖 Generated with Claude Code

Upabjojr and others added 2 commits August 1, 2026 10:22
The macOS branch of ResourceManager::setupDataPath() does not compile: it builds
the bundle path with applePath + "/", where applePath is a char array and "/" is
another array, so it is adding a pointer to a pointer. Whatever else is true of
the macOS build, it has not been attempted for a long time.

The editor's load and save dialogs start in $HOME, which on Windows is only set
if somebody has set it, so they opened on nothing. They now start in the folder
the player's own levels are saved to, which ResourceManager already resolves per
platform, and which is where a level being loaded or saved from the editor
belongs anyway. That was getEnv()'s only caller, and its comment already said it
should be doing something else on Windows.

Neither of these is verified on the platform it concerns. There is no macOS in
the build system beyond a single if(WIN32 OR APPLE), and the Windows CI is an
AppVeyor file pinned to a branch this fork does not have.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(cherry picked from commit db2e230)
A level file written on Windows does not load anywhere else. Its lines end with
a carriage return before the line feed, and only Windows takes that back off
when reading, so everywhere else the carriage return stays at the end of the
line. There it makes the last value on the line, or a section marker standing
alone, into something the parser does not recognise, and loading fails outright:
verified here by converting a level to CRLF, which turns into "The level file
can't be loaded". Helper::readFile now strips it, which covers every level and
every config file since they all come through there. The same level loads
cleanly afterwards, as do all twelve config files converted the same way.

Then the Windows specific parts:

The window icon is installed with SetClassLong and a handle cast to LONG. A
handle is 64 bits wide in a 64 bit build and LONG stays 32, so that does not
compile there. SetClassLongPtr is the same call in a 32 bit build.

The Discord link ran xdg-open, which is the freedesktop way and exists on
neither of the other two. It now uses ShellExecute on Windows and open on macOS.

The editor's file lists asked whether a name starts with a dot to decide whether
a file is hidden. On Windows that is an attribute of the file rather than
anything in its name, and the code that would have read it was commented out, so
the "show hidden files" checkbox did nothing. It now reads the attribute, which
needs the whole path rather than the file name.

setupOgreResources treated a path as absolute if it starts with '/', which no
absolute Windows path does. And the two places that make sure a folder ends with
a separator only accepted '/', so a path ending in a backslash got a slash added
after it.

Only the level file fix is verified: it is the only one of these that can be
reached from Linux.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(cherry picked from commit f45b427)
@tomluchowski

Copy link
Copy Markdown
Owner

MacOs , MacOs did anyone get the game running on that platform ?

#endif
// The result is worth looking at only to say so: there is nothing to fall back on,
// and the game is on its way out by the time this runs.
if(std::system(command.c_str()) != 0)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I am not too smart, but this branch will not compile on WINDOWS OS, because std::string command would end up being not declared anywhere .......

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude reported that compiling on Windows would require more extensive code changes. I told him to stop. Do you want to go on?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I have my local code augmentation for my Windows installment in here, otherwise I wouldn't ship my binary version of ODP for Windows. Just for the sake of sanity , move the declaration of std::string command above the preprocessor '#if' ... I will cope with rest. BTW: are you aware of this little site : https://opendungeons.org/ ? ( with binary for W included ) .

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I wasn't aware of that site. I've asked Claude to fix building for Windows and test building for Mac. After he finishes, I will push the changes here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude has now verified this directly: it compile-checked every translation unit of the game for Windows with MinGW-w64 (GCC 16, both x86_64 and i686) against real Windows headers plus Ogre 13.6.5 / CEGUI / SFML 2.6 headers. AdvertMode.cpp compiles cleanly as it is: command is only declared and used inside the non-Windows #else branch — on Windows the call goes through ShellExecuteA instead, so nothing ends up undeclared. If you would still prefer std::string command hoisted above the #if for readability, happy to move it.

Two commits are now pushed to this branch:

  • f876104 — the two remaining fixes needed to compile on macOS (verified: the game now compiles and links on a Mac, against Ogre 13.6.5 / CEGUI / SFML 2.6.2 / OIS 1.5.1 built from the versions pinned in snap/snapcraft.yaml);
  • 9ccaa13 — one real Windows issue the sweep did find: #define NOMINMAX collides with MinGW's libstdc++, which predefines it, and that warning is fatal under the project's default -Werror. Both defines are now guarded with #ifndef.

Caveats: this was per-file compilation with MinGW, not a full Windows link, and the two StackTraceWin* files could not be checked here (they need bfd.h / MSVC).

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YEah I usually build my W binary bundle with MSVC ...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have Windows. Cannot test it.

Francesco Bonazzi and others added 3 commits August 6, 2026 11:56
Helper::toString calls with a size_t argument are ambiguous on macOS for
the same reason they already were on OpenBSD: size_t is unsigned long,
which is neither of the fixed-width 64 bit types there. Enable the same
size_t overload on Apple.

StackTraceUnix relies on struct sigcontext and the deprecated ucontext
routines, neither of which macOS provides; give macOS the stub stack
trace instead.

Verified: the full game now compiles and links on macOS (Ogre 13.6.5,
CEGUI, SFML 2.6.2, OIS 1.5.1 built from the versions pinned in
snap/snapcraft.yaml).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
MinGW's libstdc++ already defines NOMINMAX, so defining it again warns,
and the game compiles with warnings treated as errors by default. Only
define NOMINMAX and WIN32_LEAN_AND_MEAN when nothing else has.

With this, every game translation unit compiles for Windows: checked
with MinGW-w64 GCC (x86_64 and i686) against real Windows headers,
Ogre 13.6.5, CEGUI and SFML 2.6.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
On Apple the data path came only from the bundle layout, and nothing at
all set the plugins.cfg path: outside a real .app bundle the game could
not find its data and never loaded a render system. The current-folder
overrides (data and plugins.cfg in "." win over the installed ones)
and the plugins.cfg resolution were sitting in the Windows-and-Linux
branch even though they are not platform specific; run them on every
platform.

Verified on macOS: the game now starts, loads all its data and runs a
level in --server mode. (Graphics could not be exercised on the build
machine, a VM with no GL driver; the rendering path is unchanged and
assumes working drivers.)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Upabjojr

Copy link
Copy Markdown
Author

Partially, yes — and it's on record in #31: I built the tree on macOS (-DOD_BUILD_TESTING=ON) and ran the whole client/server integration suite through ctest there, which is what the second commit of that PR (68af8d1e, "Fix building and running the tests on macOS") was for. So the engine, the headless server and the network client all compile and run on macOS today.

What nobody has done yet is run the full graphical game on a Mac — that additionally needs Ogre and CEGUI built for macOS, and I only had the machine long enough to validate the headless side. The fixes in this PR (the setupDataPath pointer arithmetic, open for the Discord link) are prerequisites for that day, not a claim that it already happened.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants